add support for no-index, requirements files, and repo paths #40
Conversation
georgeliaw
force-pushed
the
requirements-file-support
branch
2 times, most recently
from
c04abc4
to
d9a65f0
Compare
georgeliaw
force-pushed
the
requirements-file-support
branch
from
d9a65f0
to
36d7308
Compare
pex/pex_rules.bzl
Outdated
| for dep in ctx.attr.deps: | ||
| if hasattr(dep.py, "repos"): | ||
| repos += dep.py.repos | ||
| for file in egg_file_types.filter(ctx.files.repos): |
There was a problem hiding this comment.
Would it work to allow source distributions in repos along with eggs and wheels? That could be very convenient at times.
There was a problem hiding this comment.
Also technically one can specify https URLs pointing at a package repository with --repos. It's not a great practice with regard to Bazel, but I know people will want to do that... Thoughts?
There was a problem hiding this comment.
@benley added support for source distributions as well as cache disabling.
Have some mixed feelings about supporting URLs for the same reason that it's not exactly great practice. I'll think on the issue a bit this week.
pex/BUILD
Outdated
| @@ -22,7 +22,7 @@ genrule( | |||
| # Workaround really long shebang lines breaking on linux: | |||
| # Use a /tmp path, but keep the actual venv inside the bazel outdir. | |||
| # Avoids having to worry about cleanup, even if sandboxing is off. | |||
| TMPF=$$(mktemp) | |||
| TMPF=$$(mktemp 2>/dev/null || mktemp -t tmp) | |||
There was a problem hiding this comment.
Wow, I wonder how this ever worked on MacOS - did mktemp's behaviour change in a recent macos release?
There was a problem hiding this comment.
@benley yeah this was a weird one, it appears the behavior was fixed to match gnu in OS X 10.11, but older versions of OS X require the -t flag. Only encountered it due to a coworker running into this issue.
https://unix.stackexchange.com/questions/30091/fix-or-alternative-for-mktemp-in-os-x
|
@benley gave the URL Anything nicer would probably require writing new workspace rules for like For the time being, we can consider the PR ready to merge unless there are any changes you would like me to make. EDIT: I see we both seem to agree on new workspace rules being the nicer option to implement from looking at #20. We can probably look at that at a later point in time. |
|
@benley I just recalled that requirements files themselves can be used to point to other repo urls, so technically this is already supported. Again, obviously not the greatest practice when it comes to Bazel, but the use-case is available. Pip since version 8.0 also supports hash checking, which we could try enforcing to improve the build determinism: This technically means we wouldn't have to go down the workspace rule route. What are your thoughts on this? |
|
Oh cool, that is a good point. I completely forgot about this PR, sorry about that! |
Needed to update the
pex_binaryrules to keep the clutter down in the BUILD files and make them easier to maintain. Figured I should open a PR in case you would like to pull this into master.